feat: move restartOnError to run options #1965
Conversation
DaveHanns
requested review from
B4nan
and removed request for
janbuchar and
fnesveda
github-actions
bot
added
the
t-console
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Comment @cursor review or bugbot run to trigger another review on this PR
| description: '' | ||
| nullable: true | ||
| defaultRunOptions: | ||
| oneOf: |
There was a problem hiding this comment.
Question: @B4nan, why do we declare the nullable fields in some cases like this ?
It results in a bit weird notation in docs, see screenshot.
There was a problem hiding this comment.
no idea, the openapi schema was generated from apiary some time ago, could have been a quirk in the conversion script
There was a problem hiding this comment.
What is the actual type accepted by the API? Is it "DefaultRunOptions or null"?
I'm asking so that I can check out if there's a better way to express this. But you're right that this looks suspicious.
There was a problem hiding this comment.
Good question ... it actually is not nullable, as setting DefaultRunOptions to null will fail both creation and update of the Actor.
So it case of update and create schema, it should be only optional, but not nullable 🤔
I guess not listing it in required but not setting it as nullable should be good enough.
Will try later.
There was a problem hiding this comment.
Done. Now it should better reflect the behaviour of the API.
Side note: I'm planning to revisit the DefaultRunOptions and make API accept partial DefaultRunOptions, so that user can specify only the defaults they actually want to customize and get our internal defaults for the rest.
There was a problem hiding this comment.
Cool! Thanks for investigating 🙂
|
Preview for this PR was built for commit |
|
Preview for this PR was built for commit |
|
Preview for this PR was built for commit |
|
Preview for this PR was built for commit |
Documentation adjustments according to changes based on https://app.zenhub.com/workspaces/platform-team-5f6454160d9f82000fa6733f/issues/gh/apify/apify-core/
Core: https://github.com/apify/apify-core/pull/23381
Worker: https://github.com/apify/apify-worker/pull/1515
Shared: apify/apify-shared-js#546
JS Client: apify/apify-client-js#760
Py Client: apify/apify-client-python#508